Add `balance_before` and `balance_after` to transactions by rameerez · Pull Request #27 · rameerez/usage_credits

rameerez · 2026-01-15T19:27:26Z

Implements the "balance after transaction" feature requested in #8 — adding running balance information to transactions, similar to how banking apps display balance after each transaction.

Key additions:

transaction.balance_after — the wallet balance immediately after this transaction
transaction.balance_before — the wallet balance immediately before this transaction
transaction.formatted_balance_after — formatted output using configured credit formatter

Why This Approach?

Goal

Add banking-like running balance to transaction history with:

✅ Minimal footprint — only 36 lines of production code added
✅ No migrations — uses existing metadata JSON column
✅ Full backwards compatibility — old transactions return nil gracefully
✅ Production-ready safety — ACID-compliant for money-like assets

Constraints Considered

No database schema changes — Users shouldn't need to run migrations
Backwards compatibility — Existing transactions must continue to work
Accuracy over convenience — For banking-like systems, data must be correct in ALL edge cases
Atomic operations — Credit operations must be all-or-nothing

Design Decisions

Why store in metadata vs. a new column?

No migration required
JSON column already exists and is used for audit data
Backwards compatible — old transactions simply have no balance info
Consistent with existing pattern of storing audit data in metadata

Why store BOTH balance_before AND balance_after?

Initially, I implemented balance_before as calculated: balance_after - amount. This works 99% of the time, but breaks in edge cases:

# With allow_negative_balance enabled:
# balance_before = 10, amount = -25, balance_after = 0 (floored)
# Calculated balance_before = 0 - (-25) = 25 ❌ (should be 10)

For a banking-like system handling money-equivalent assets, calculated values that can be wrong in edge cases are unacceptable. Explicit storage guarantees correctness.

Why update metadata after transaction creation (two writes)?

The balance can only be accurately computed AFTER the transaction exists (the credits method queries all transactions). Options considered:

Compute upfront (previous_balance ± amount) — Simpler formula but could diverge from actual credits calculation in edge cases
Update after create — Uses actual computed balance, guarantees consistency with wallet.balance column

Chose option 2 for safety. The extra write is negligible and fully protected by the database transaction.

Safety Analysis

ACID Compliance

Property	Implementation
Atomicity	All operations inside `with_lock do...end` — if `update!` fails, entire transaction rolls back
Consistency	`balance_after` always equals `wallet.balance` at commit time
Isolation	Row-level `SELECT FOR UPDATE` lock prevents concurrent modifications
Durability	Standard Rails/ActiveRecord guarantees

Callback Safety

Callbacks are executed inside execute_safely which rescues all errors — callbacks can never break credit operations or cause partial commits.

All Entry Points Covered

Audited every code path that creates transactions:

Entry Point	Method	Coverage
`give_credits`	→ `add_credits`	✅
`spend_credits_on`	→ `deduct_credits`	✅
Credit pack purchase	→ `add_credits`	✅
Subscription credits (trial/bonus/regular)	→ `add_credits`	✅
Subscription upgrade	→ `add_credits`	✅
Fulfillment job	→ `add_credits`	✅
Refunds	→ `deduct_credits`	✅

Test Coverage

Added 27 comprehensive tests covering:

Basic accessor functionality
Storage during give_credits, deduct_credits, spend_credits_on
Mathematical consistency (balance_before + amount = balance_after)
Edge cases:
- Nil metadata handling
- Zero starting balance
- Exact deduction to zero
- Large credit amounts (1 billion)
- Negative balance scenarios (allow_negative_balance)
- Transaction chain consistency
- Metadata persistence across reloads
- Concurrent metadata updates

Test Results: 700 tests, 1587 assertions, 0 failures, 0 errors

Usage Example

# Get transaction history with running balance (like a bank statement)
user.credit_history.each do |tx|
  puts "#{tx.created_at.strftime('%Y-%m-%d')}: #{tx.formatted_amount}"
  puts "  Balance: #{tx.balance_before} → #{tx.balance_after}"
end

# Output:
# 2024-12-16: +1000 credits
#   Balance: 0 → 1000
# 2024-12-26: +500 credits
#   Balance: 1000 → 1500
# 2025-01-14: -50 credits
#   Balance: 1500 → 1450

Files Changed

File	Changes
`lib/usage_credits/models/transaction.rb`	+24 lines — accessors and formatted helper
`lib/usage_credits/models/wallet.rb`	+12 lines — store balance info in metadata
`test/models/usage_credits/transaction_test.rb`	+392 lines — 27 new tests

Checklist

No migrations required
Backwards compatible (old transactions return nil)
All 700 tests pass
ACID-compliant implementation
All transaction creation paths covered
Edge cases tested (negative balance, zero balance, large amounts)
Follows existing code patterns and style

Closes #8

🤖 Generated with Claude Code

Store running balance information in transaction metadata for audit trail, similar to banking apps that show balance after each transaction. - Add balance_after and balance_before accessors to Transaction model - Store both values in metadata during add_credits and deduct_credits - Add formatted_balance_after helper method - Add 27 comprehensive tests covering edge cases No migrations required - uses existing metadata JSON column. Fully backwards compatible - returns nil for old transactions. Closes #8 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

claude · 2026-01-15T19:30:48Z

Code Review for PR #27: Add balance_before and balance_after to transactions

Summary

This PR implements a banking-like running balance feature for transactions, adding balance_after and balance_before fields stored in transaction metadata. Overall, this is a well-designed and thoroughly tested implementation that follows the existing codebase patterns.

Strengths

Code Quality

Minimal footprint: Only 36 lines of production code
Backwards compatible: Returns nil for old transactions, no migration required
Excellent test coverage: 27 comprehensive tests covering edge cases
Clean implementation following existing patterns

Design Decisions

ACID compliance: Operations inside with_lock ensure atomicity
Explicit storage: Storing both balance_before and balance_after avoids calculation errors in edge cases
Proper timing: Balance information stored after transaction creation using actual computed values

Testing

Thorough edge case coverage: nil metadata, negative balances, zero balances, large amounts, interleaved operations
Validates mathematical consistency
Tests persistence across reloads and metadata updates
Tests all entry points

Areas for Consideration

1. Race Condition in Two-Write Pattern (Medium Priority)

Location: lib/usage_credits/models/wallet.rb:187-190 and 287-290

The implementation uses two database writes. Between the first and second write, the transaction exists without balance metadata. If the second update! fails, transactions would be created without balance information.

Recommendation: Consider computing balance upfront and including in initial create!, or add defensive backfill logic.

2. Performance Consideration (Low Priority)

The current implementation calls credits method twice per transaction, which executes a complex SQL query each time. For high-volume operations, this adds 2x query overhead.

Recommendation: Cache the balance calculation within the lock block, or use the balance column directly since it is kept in sync.

3. Metadata Merge Ordering (Low Priority)

The code merges balance information into existing metadata. If custom metadata happens to include balance_before or balance_after keys, they would be overwritten silently.

Recommendation: Consider namespacing system metadata or documenting that these keys are reserved.

Security

No security concerns identified. Uses parameterized queries, proper escaping, and row-level locks.

Test Coverage

The test suite is exceptionally comprehensive with 27 tests covering all major scenarios and edge cases.

Suggested additions (optional):

Test what happens if update! fails in the two-write pattern
Performance test with large transaction histories
Concurrent transaction test

Documentation

The PR description is excellent with clear explanation of design decisions, trade-offs, safety analysis, and usage examples.

Minor suggestion: Add a usage example to README.md showing the new feature in action.

Recommendation

APPROVE with minor suggestions

This is a high-quality PR with excellent testing and documentation. The two-write pattern is the only notable concern, but it is within acceptable risk since operations are within a lock, old transactions gracefully return nil, and the feature is additive.

Great work on this feature!

…r` features

- Add comments explaining atomicity guarantee (with_lock ensures rollback) - Document that balance_before/balance_after keys are system-reserved - Add running balance documentation to README Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

rameerez · 2026-01-15T19:37:19Z

Addressing the review points:

1. Two-Write Pattern (Medium Priority)

Good catch on the pattern, but this is actually zero risk rather than "acceptable risk":

The with_lock block in Rails wraps everything in a single database transaction. If the update! fails:

An exception is raised
The entire DB transaction rolls back (including the create!)
No orphaned transaction can exist

However, I agree the code could be clearer. I've added an explanatory comment in d9dcda6:

# Store balance information in transaction metadata for audit trail.
# Note: This update! is in the same DB transaction as the create! above (via with_lock),
# so if this fails, the entire transaction rolls back - no orphaned records possible.

2. Metadata Merge Ordering (Low Priority)

The current behavior is intentional - we want to overwrite any user-supplied balance_before/balance_after keys. The system must be authoritative about balance values; letting user-supplied values through would be a data integrity risk.

This follows the same pattern used elsewhere in the codebase (e.g., executed_at, gem_version are always system-set).

I've added a comment documenting this is intentional:

# We intentionally overwrite any user-supplied balance_before/balance_after keys
# to ensure system-set values are authoritative.

rameerez and others added 2 commits January 15, 2026 19:31

Add docs in the README for the new balance_before and `balance_afte…

e8f6af6

…r` features

rameerez merged commit fa1b336 into main Jan 15, 2026
22 of 23 checks passed

rameerez deleted the feature/balance-after-transaction branch January 15, 2026 19:42

rameerez changed the title ~~Add balance_before and balance_after to transactions~~ Add balance_before and balance_after to transactions Jan 15, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `balance_before` and `balance_after` to transactions#27

Add `balance_before` and `balance_after` to transactions#27
rameerez merged 3 commits intomainfrom
feature/balance-after-transaction

rameerez commented Jan 15, 2026 •

edited

Loading

Uh oh!

claude bot commented Jan 15, 2026

Uh oh!

rameerez commented Jan 15, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

rameerez commented Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why This Approach?

Goal

Constraints Considered

Design Decisions

Why store in metadata vs. a new column?

Why store BOTH balance_before AND balance_after?

Why update metadata after transaction creation (two writes)?

Safety Analysis

ACID Compliance

Callback Safety

All Entry Points Covered

Test Coverage

Usage Example

Files Changed

Checklist

Uh oh!

claude bot commented Jan 15, 2026

Code Review for PR #27: Add balance_before and balance_after to transactions

Summary

Strengths

Code Quality

Design Decisions

Testing

Areas for Consideration

1. Race Condition in Two-Write Pattern (Medium Priority)

2. Performance Consideration (Low Priority)

3. Metadata Merge Ordering (Low Priority)

Security

Test Coverage

Documentation

Recommendation

Uh oh!

rameerez commented Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

1. Two-Write Pattern (Medium Priority)

2. Metadata Merge Ordering (Low Priority)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

rameerez commented Jan 15, 2026 •

edited

Loading

rameerez commented Jan 15, 2026 •

edited

Loading